assets: Add shared asset tooling - #1225

Could we make this method enforce full provenance rather than leaving that distinction to callers?

VerifyProofResponse.Valid does not necessarily mean that a proof history is rooted at issuance. In Taproot Assets v0.8.3:

• tapd's VerifyProof calls File.Verify and does not bind GenesisPoint (rpcserver.go:2159).
• File.Verify starts the first proof with prev == nil (verifier.go:1565).
• In that state, Proof.Verify accepts a non-nil ChallengeWitness as an ownership proof (verifier.go:1227). This is intentional behavior (proof_test.go:975).

Because the deposited asset uses an OP_TRUE script key, satisfying that ownership challenge is trivial. An attacker could anchor a leaf with the target genesis identity and claimed amount under the expected public Bitcoin contract. The current chain-inclusion, commitment, outpoint, amount, internal-key and sibling checks could then pass without proving legitimate issuance or transition history.

I think VerifyProofFile should provide the safe abstraction promised by its name:

1. Snapshot RawProofFile once and verify and decode that same immutable content.
2. Reject empty files and non-nil ChallengeWitness values.
3. Require the first asset to satisfy IsGenesisAsset().
4. Apply those requirements recursively to every proof file in AdditionalInputs.
5. Call tapd to verify the genesis reveal, group keys, chain inclusion, commitments and state transitions, then perform the existing terminal contract bindings.

IsGenesisAsset() is only a structural precondition; tapd's cryptographic verification is still required. Conversely, tapd verification alone permits the ownership-proof shortcut. Please add regressions for a standalone ownership proof and an ownership-rooted AdditionalInputs file.

Confirmation depth and current unspent status are separate live-chain properties. If callers should have one safe go/no-go operation, a higher-level VerifyDeposit can encapsulate those checks while keeping VerifyProofFile focused on provenance and exact contract binding.

I would also consider removing or unexporting the commitment-only VerifyProof before merge (kit.go:515). Keeping an attractively named unsafe alternative makes it easy for Loop server to bypass the safe path accidentally.

What policy should asset loop-in use?

Do we keep LegacyDepositV0 for drafts written before?

The method creates a private copy and sends that to tapd, but afterward decodes the caller-owned original at line 497. Mutation while the RPC is in flight can therefore make Loop consume bytes tapd did not verify.

Make one immutable snapshot and both verify and decode that exact snapshot. Add a verifier mock that mutates the caller’s original buffer to prevent regression.

The deposit kit collapses four independent key roles into two.

Kit stores only funderKey and coSignerKey. The same pair is then:

• aggregated into the deposit’s MuSig2 internal key at assets/deposit/kit.go:103-110;
• used for the deposit refund script at assets/deposit/kit.go:138-150;
• passed to the HTLC as its sender and receiver keys at assets/deposit/kit.go:273-285;
• aggregated again as the HTLC internal key at assets/htlc/swapkit.go:285-301;
• used directly in the HTLC success and timeout leaves at assets/htlc/swapkit.go:261-283.

This cannot represent the intended four direction-neutral roles:

• SenderInternalPubKey and ReceiverInternalPubKey form the cooperative MuSig2 internal key.
• SenderScriptKey authorizes the unilateral deposit refund and HTLC timeout paths.
• ReceiverScriptKey authorizes the HTLC preimage-success path.

“Sender” and “receiver” are defined by the on-chain asset direction: for Asset Loop In, Loop is the sender and Loop server is the receiver; for Asset Loop Out, the mapping is reversed. This is also the model already used by Bitcoin HTLCs in loopdb/loop.go:13-35 and swap/htlc.go:535-615.

There is no correct way for callers to map four keys into the current two-key constructor. Passing the internal keys makes the unilateral scripts use the wrong keys; passing the script keys makes the cooperative aggregate use the wrong keys. This changes the deposit address, HTLC output, control blocks, signing responsibilities, and recovery paths.

Please preserve all four roles as separate immutable constructor inputs. Aggregate only the sender and receiver internal keys, use the sender script key in refund/timeout leaves, and use the receiver script key in the preimage-success leaf. The shared SwapKit policy should derive and validate that same mapping for both Loop In and Loop Out.

Witness methods will sign asset-burning outputs.

Both SwapKit.validateSweep and deposit.Kit.validateSweep verify only that the selected input is the expected asset anchor. assets/htlc/swapkit.go:618-708 and assets/deposit/kit.go:583-665 return without inspecting any transaction output or binding the PSBT to a validated Taproot Asset transition.

The PSBT is then signed by CreatePreimageWitness at assets/htlc/swapkit.go:812-815, CreateTimeoutWitness at assets/htlc/swapkit.go:883-886, and deposit CreateTimeoutWitness at assets/deposit/kit.go:730-733. SIGHASH_DEFAULT commits to all outputs, but that does not make them asset-safe—the helper signs whatever outputs the caller supplied.

Bitcoin consensus validates the anchor spend, not Taproot Asset conservation. A transaction can therefore spend the expected P2TR anchor while omitting or replacing the destination asset commitment. Once confirmed, this destroys the active asset’s valid transition history and can also burn passive assets co-anchored in the same UTXO.

The tests currently demonstrate this gap: the HTLC fixture’s only output is a bare OP_TRUE output at assets/htlc/swapkit_test.go:744-747, and the deposit fixture does the same at assets/deposit/kit_test.go:353-355; both are nevertheless signed successfully.

Because this kit is intended to hide Taproot Asset internals from callers, output correctness should not be an implicit caller obligation. Before releasing a signature or witness, the shared API must bind the exact PSBT to the exact validated virtual transition and verify:

• the active asset ID and amount;
• the intended receiver asset script key;
• the anchor output index, value, internal key, sibling, commitment root, and resulting pkScript;
• all required change and passive-asset outputs;
• that the Bitcoin outputs being signed are exactly those produced by the validated Taproot Asset transition.

Tests should construct a real asset-preserving transition, then verify that mutations to the destination output, amount, keys, sibling, commitment, or passive-asset carry-forward are rejected before the signer is invoked.

Medium — NewHtlcAddr and CreateHtlcVpkt describe different asset commitments.

This request leaves both version fields at their protobuf zero values. With Taproot Assets v0.8.3, that selects Asset V0 and leaves the address version unspecified; tapd currently resolves the latter to Address V1. In contrast, CreateHtlcVpkt explicitly creates VPacket V1 and Asset V1 outputs at assets/htlc/swapkit.go:373-397.

The asset version is serialized into the asset leaf, so changing it changes the Taproot Asset commitment root and ultimately the Bitcoin output key. The address returned here and the VPacket returned by the same SwapKit therefore do not construct the same HTLC output. Relying on tapd's unspecified address-version default also leaves part of the contract outside the immutable policy.

Please freeze every relevant version in the policy—at minimum the address, asset, VPacket, and TapCommitment versions—and use those values consistently in both address requests and VPacket construction. Proof-file and transition-proof versions accepted by validation should be frozen there as well. Validate the versions returned by tapd, and add a test showing that the decoded address and the funded VPacket derive the same output key.

(also applies to assets/deposit/kit.go:212)

Low — distinct-key validation compares full points even though the contracts use x-only keys.

btcec.PublicKey.IsEqual distinguishes P from -P, but BIP340/Taproot serializes both points to the same x-only public key. A caller can therefore provide opposite-parity encodings of the same key and bypass the "keys must differ" check, even though one underlying signing key controls both nominal roles in the resulting contract.

Please compare schnorr.SerializePubKey encodings after parsing and apply the same x-only distinctness rule to every policy role that must be independently controlled. Add a regression test using P and -P.

Low — VerifyProofFile duplicates oversized proofs before enforcing Taproot Assets' size limit.

The first operation on the raw proof is an unbounded append copy. Tapd's VerifyProof RPC applies proof.CheckMaxFileSize, but that happens only after this allocation and after the buffer crosses the pluggable verifier boundary. An oversized untrusted proof can therefore force a second allocation of the full payload before it is rejected.

Please capture the input slice, call proof.CheckMaxFileSize before allocating the immutable snapshot, and then use that one snapshot for both verification and decoding. Oversized input should be rejected before invoking the verifier.

Low — the RPC request exposes mutable storage from the supposedly immutable deposit kit.

d.assetID[:] is a slice backed directly by the array stored in Kit. Because AddressProofClient is a pluggable interface, an implementation or interceptor can retain and mutate request.AssetId, changing d.assetID after construction. On the HTLC path this can also make the address response disagree with the SwapKit that was created before the RPC call.

Please copy AssetId into request-owned storage before crossing the interface boundary in both address methods. A mock that mutates and retains the request slice should demonstrate that subsequent kit operations remain unchanged.

Non-blocking documentation request — clarify that CreateHtlcVpkt returns a funding template.

The zero-valued interactive split-root placeholder at assets/htlc/swapkit.go:381-388 is correct for tapd's funding pipeline, but it is not valid as a final VPacket: PrepareOutputAssets rejects interactive zero-valued outputs. FundVirtualPsbt first converts the template to allocations, replaces the placeholder amount with actual change, and omits the local output entirely for a full-value interactive transfer.

Could the method documentation state that its result must pass through FundVirtualPsbt and must not be validated or committed as a final VPacket? Renaming it to something like CreateHtlcFundingTemplate would make the contract even clearer, but documentation is sufficient here. A funded-template test covering both exact-value and change cases would preserve this subtle dependency.

Low — the policy permanently fixes a shared OP_TRUE asset script key.

CreateOpTrueLeaf has no contract-specific input, so all deposits and HTLCs use the same asset script key. This matches the historical prototype and is valid for individual swaps, but it prevents multiple outputs for the same asset from being constructed in one transaction and precludes script-key-addressed proof delivery without another disambiguation mechanism.

Please document whether those limitations are intentional for this immutable policy. If same-transaction batching or script-key-addressed proof delivery is required later, a new policy will need a deterministic contract-specific discriminator while retaining OP_TRUE as its spendable script path.